test: Run Guidepup tests in CI #40
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull request overview
This PR enables Guidepup tests to run successfully in CI by fixing path resolution issues, adding timeouts for test stability, and configuring separate CI jobs for macOS and Windows platforms.
Changes:
- Fixed file path resolution in test fixtures to correctly serve static files
- Added explicit waits and focus actions to improve test reliability with screen readers
- Switched Playwright configuration to use Firefox for testing the ariaNotify polyfill
- Split CI workflow into three separate jobs: web-test-runner, guidepup-macos, and guidepup-windows
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/guidepup/voiceover.spec.mjs | Fixed path resolution, removed redundant navigation commands, added waits for stability, commented out failing assertion |
| tests/guidepup/nvda.spec.mjs | Fixed path resolution and added waits for test stability |
| playwright.config.mjs | Changed browser from Edge to Firefox to test the polyfill |
| package.json | Added Firefox installation to test:guidepup script |
| .github/workflows/test.yml | Split test job into three separate jobs with platform-specific Guidepup setup |
Comments suppressed due to low confidence (2)
tests/guidepup/voiceover.spec.mjs:63
- Using hardcoded timeouts with waitForTimeout is an anti-pattern in Playwright tests as it makes tests flaky and slower. Consider using waitFor conditions or polling assertions instead, such as waiting for specific elements or states.
await page.waitForTimeout(500);
tests/guidepup/nvda.spec.mjs:92
- Using hardcoded timeouts with waitForTimeout is an anti-pattern in Playwright tests as it makes tests flaky and slower. Consider using waitFor conditions or polling assertions instead, such as waiting for specific elements or states.
await page.waitForTimeout(500);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates Guidepup tests so they run successfully, and runs them in CI. This can catch regressions that may occur as we develop the polyfill, increasing confidence in changes.
I updated the branch policy’s required checks accordingly.